cmd/zetaclientd/start.go (1)

Line range hint 254-266: Consider addressing the disabled zeta supply checker

The TODO comment indicates that the zeta supply checker is disabled due to performance concerns with GRPC queries. This might be worth revisiting now that we're improving synchronization performance.

Would you like me to:

1. Open a GitHub issue to track the re-enablement of the supply checker?
2. Propose an implementation that addresses the performance concerns?

zetaclient/orchestrator/orchestrator_test.go (1)

591-591: Consider enhancing mock client initialization.

While the mock client is correctly added to the Orchestrator struct, the current initialization could be improved to better support testing different scenarios.

Consider this enhancement:

-		cometbftClient: &cometbft_rpc_client_mock.Client{},
+		cometbftClient: func() *cometbft_rpc_client_mock.Client {
+			mockClient := &cometbft_rpc_client_mock.Client{}
+			// Initialize with default expectations
+			mockClient.On("Start").Return(nil)
+			return mockClient
+		}(),

zetaclient/orchestrator/orchestrator.go (5)

321-330: Simplify Operator Balance Update Logic

The current logic for updating the operator balance includes unnecessary checks and can be streamlined for clarity and efficiency.

diff --git a/zetaclient/orchestrator/orchestrator.go b/zetaclient/orchestrator/orchestrator.go
--- a/zetaclient/orchestrator/orchestrator.go
+++ b/zetaclient/orchestrator/orchestrator.go
@@ -324,13 +324,9 @@
 } else {
     diff := oc.lastOperatorBalance.Sub(balance)
-    if diff.GT(sdkmath.NewInt(0)) && diff.LT(sdkmath.NewInt(math.MaxInt64)) {
+    if diff.IsPositive() {
         oc.ts.AddFeeEntry(bn, diff.Int64())
         oc.lastOperatorBalance = balance
     }
 }

---

341-344: Enhance Error Logging for Clarity

Improve the error message when failing to retrieve pending cross-chain transactions to provide clearer context for troubleshooting.

-oc.logger.Error().Err(err).Msgf("runScheduler: GetPendingCctxsWithinRatelimit failed")
+oc.logger.Error().Err(err).Msg("failed to retrieve pending cross-chain transactions within rate limit")

🧰 Tools

🪛 GitHub Check: codecov/patch

[warning] 341-344: zetaclient/orchestrator/orchestrator.go#L341-L344
Added lines #L341 - L344 were not covered by tests

---

358-360: Provide Additional Context in Error Messages

When logging errors during signer resolution, include the chain ID and relevant details to aid in debugging.

 oc.logger.Error().Err(err).
     Int64(logs.FieldChain, chainID).
-    Msg("runScheduler: unable to resolve signer")
+    Msgf("unable to resolve signer for chain ID %d", chainID)

🧰 Tools

🪛 GitHub Check: codecov/patch

[warning] 358-360: zetaclient/orchestrator/orchestrator.go#L358-L360
Added lines #L358 - L360 were not covered by tests

---

364-369: Avoid Redundant Error Handling in Observer Resolution

The error handling logic duplicates context provided by the errors.Wrapf function. Streamlining this can enhance readability.

 if err != nil {
-    oc.logger.Error().Err(err).
-        Int64(logs.FieldChain, chainID).
-        Msg("runScheduler: unable to resolve observer")
+    oc.logger.Error().Err(err).Msgf("unable to resolve observer for chain ID %d", chainID)
     continue
 }

🧰 Tools

🪛 GitHub Check: codecov/patch

[warning] 364-369: zetaclient/orchestrator/orchestrator.go#L364-L369
Added lines #L364 - L369 were not covered by tests

---

299-406: Increase Test Coverage for New Scheduler Logic

The new event-driven scheduler implementation lacks test coverage, as indicated by static analysis. Enhancing tests will improve reliability and prevent regressions.

Would you like me to assist in creating unit tests for the runScheduler method to ensure thorough coverage?

🧰 Tools

🪛 GitHub Check: codecov/patch

[warning] 299-302: zetaclient/orchestrator/orchestrator.go#L299-L302
Added lines #L299 - L302 were not covered by tests

---

[warning] 309-329: zetaclient/orchestrator/orchestrator.go#L309-L329
Added lines #L309 - L329 were not covered by tests

---

[warning] 333-338: zetaclient/orchestrator/orchestrator.go#L333-L338
Added lines #L333 - L338 were not covered by tests

---

[warning] 341-344: zetaclient/orchestrator/orchestrator.go#L341-L344
Added lines #L341 - L344 were not covered by tests

---

[warning] 347-350: zetaclient/orchestrator/orchestrator.go#L347-L350
Added lines #L347 - L350 were not covered by tests

---

[warning] 353-356: zetaclient/orchestrator/orchestrator.go#L353-L356
Added lines #L353 - L356 were not covered by tests

---

[warning] 358-360: zetaclient/orchestrator/orchestrator.go#L358-L360
Added lines #L358 - L360 were not covered by tests

---

[warning] 364-369: zetaclient/orchestrator/orchestrator.go#L364-L369
Added lines #L364 - L369 were not covered by tests

---

[warning] 373-379: zetaclient/orchestrator/orchestrator.go#L373-L379
Added lines #L373 - L379 were not covered by tests

---

[warning] 383-384: zetaclient/orchestrator/orchestrator.go#L383-L384
Added lines #L383 - L384 were not covered by tests

---

[warning] 388-401: zetaclient/orchestrator/orchestrator.go#L388-L401
Added lines #L388 - L401 were not covered by tests

---

[warning] 406-406: zetaclient/orchestrator/orchestrator.go#L406
Added line #L406 was not covered by tests

Configuration used: .coderabbit.yaml
Review profile: CHILL

#!/bin/bash
# Description: Verify how the CometBFT client is used in the orchestrator package

# Check for block subscription usage
ast-grep --pattern 'func (o *Orchestrator) runScheduler() {
  $$$
  Subscribe($$$)
  $$$
}'

# Check for proper cleanup
ast-grep --pattern 'func (o *Orchestrator) Stop() {
  $$$
  Stop()
  $$$
}'

#!/bin/bash
# Let's try a broader search to understand the orchestrator's implementation

# Find orchestrator files
fd orchestrator

# Check how CometBFT client is used in orchestrator
rg -A 5 "CometClient|cometClient" 

# Look for block subscription patterns
rg -A 5 "Subscribe|EventsClient|BlockClient"

# Check orchestrator struct definition
ast-grep --pattern 'type Orchestrator struct {
  $$$
}'